[lldb-dap] Address a unit test race condition during initialization. #167981
Conversation
During the initialization sequence in our tests the first 'threads' response sould only be kept if the process is actually stopped, otherwise we will have stale data. In VSCode, during the debug session startup sequence immediately after 'configurationDone' a 'threads' request is made. This initial request is to retrieve the main threads name and id so the UI can be populated. However, in our tests we do not want to cache this value unless the process is actually stopped. We do need to make this initial request because lldb-dap is caching the initial thread list during configurationDone before the process is resumed. We need to make this call to ensure the cached initial threads are purged. I noticed this in a CI job for another review (https://github.com/llvm/llvm-project/actions/runs/19348261989/job/55353961798) where the tests incorrectly failed to fetch the threads prior to validating the thread names.
|
@llvm/pr-subscribers-lldb Author: John Harrison (ashgti) ChangesDuring the initialization sequence in our tests the first 'threads' response sould only be kept if the process is actually stopped, otherwise we will have stale data. In VSCode, during the debug session startup sequence immediately after 'configurationDone' a 'threads' request is made. This initial request is to retrieve the main threads name and id so the UI can be populated. However, in our tests we do not want to cache this value unless the process is actually stopped. We do need to make this initial request because lldb-dap is caching the initial thread list during configurationDone before the process is resumed. We need to make this call to ensure the cached initial threads are purged. I noticed this in a CI job for another review (https://github.com/llvm/llvm-project/actions/runs/19348261989/job/55353961798) where the tests incorrectly failed to fetch the threads prior to validating the thread names. Full diff: https://github.com/llvm/llvm-project/pull/167981.diff 1 Files Affected:
diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
index ac550962cfb85..98a06715ae6a4 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
@@ -191,6 +191,11 @@ class NotSupportedError(KeyError):
class DebugCommunication(object):
+ @property
+ def is_stopped(self) -> bool:
+ """Returns True if the debuggee is stopped, otherwise False."""
+ return len(self.thread_stop_reasons) > 0 or self.exit_status is not None
+
def __init__(
self,
recv: BinaryIO,
@@ -860,7 +865,17 @@ def request_configurationDone(self):
response = self._send_recv(command_dict)
if response:
self.configuration_done_sent = True
+ stopped_on_entry = self.is_stopped
self.request_threads()
+ if not stopped_on_entry:
+ # Drop the initial cached threads if we did not stop-on-entry.
+ # In VSCode, immediately following 'configurationDone', a
+ # 'threads' request is made to get the initial set of threads,
+ # specifically the main threads id and name.
+ # We issue the threads request to mimic this pattern but in our
+ # tests we don't want to cache the result unless the process is
+ # actually stopped.
+ self.threads = None
return response
def _process_stopped(self):
|
|
✅ With the latest revision this PR passed the Python code formatter. |
🐧 Linux x64 Test Results
|
3 participants
During the initialization sequence in our tests the first 'threads' response sould only be kept if the process is actually stopped, otherwise we will have stale data.
In VSCode, during the debug session startup sequence immediately after 'configurationDone' a 'threads' request is made. This initial request is to retrieve the main threads name and id so the UI can be populated. However, in our tests we do not want to cache this value unless the process is actually stopped. We do need to make this initial request because lldb-dap is caching the initial thread list during configurationDone before the process is resumed. We need to make this call to ensure the cached initial threads are purged.
I noticed this in a CI job for another review (https://github.com/llvm/llvm-project/actions/runs/19348261989/job/55353961798) where the tests incorrectly failed to fetch the threads prior to validating the thread names.